[Kraken2] Split database from Kraken2 TheiaCoV task #608

jrotieno · 2024-09-06T21:06:17Z

This PR closes #563, closes #520 and closes #402

🗑️ This dev branch should be deleted after merging to main.

🧠 Summary

This PR addresses several issues that came up when the RSV target organism was set differently for RSV-A and RSV-B, i.e. rsv_a_kraken_target_organism = "Respiratory syncytial virus" and rsv_b_kraken_target_organism = "Human orthopneumovirus" due to the use of an old database. As the existing docker image had an embedded database, it meant that each time the Kraken database would need to be updated, it would have to be done inside the docker container, which would not be efficient. The TheiaProk workflows were already using a standalone database and a docker image free of a database, allowing for flexibility in user input for a preferred database.

We have therefore implemented the same strategy to the TheiaCoV workflows, using the standalone Kraken2 task, and a default custom built Kraken2 database comprising Human and Viral sequences.

Further, for consistency, we have made all Kraken 2 outputs to be prefixed by "Kraken2", which previously had a mix of Kraken2_ and Kraken_

⚡ Impacted Workflows/Tasks

This PR may lead to different results in pre-existing outputs: Yes
Use of an updated database may reflect changes in how taxa are identified.

This PR uses an element that could cause duplicate runs to have different results: No

🛠️ Changes

⚙️ Algorithm

We have removed the kraken2_theiacov task and instead using the kraken2_standalone task for the TheiaCoV workflows

➡️ Inputs

The TheiaCoV workflows, Illumina PE, Illumina SE, and ONT, have a default Kraken2 database that includes Human and Viral sequences from NCBI
File kraken_db = "gs://theiagen-large-public-files-rp/terra/databases/kraken2/kraken2_humanGRCh38_viralRefSeq_20240828.tar.gz"

⬅️ Outputs

All Kraken2 outputs are now prefixed with "Kraken2_"
Float? kraken2_human
Float? kraken2_human_dehosted
Float? kraken2_sc2
Float? kraken2_sc2_dehosted
String? kraken2_target_organism
String? kraken2_target_organism_dehosted
String? kraken2_target_organism_name
String? kraken2_version
File? kraken2_report
File? kraken2_report_dehosted
String? kraken2_docker
String? kraken2_database

🧪 Testing

The database was tested locally with the standalone task and the custom database and worked as expected.

Further testing was successfully done on Terra as follows:
Bacterial pathogens and TheiaProk workflows: call_kraken=true and kraken_db=gs://theiagen-large-public-files-rp/terra/databases/kraken2/kraken2_humanGRCh38_viralRefSeq_20240828.tar.gz
TheiaProk_Illumina_PE_PHB: https://app.terra.bio/#workspaces/theiagen-training-workspaces/Theiagen_Otieno_Sandbox/job_history/10eddd67-5216-4529-b355-8a14947de621
TheiaProk_ONT_PHB: https://app.terra.bio/#workspaces/theiagen-training-workspaces/Theiagen_Otieno_Sandbox/job_history/7206c5d6-fd58-448c-becf-1abb4ed27c5c

Viral Pathogens and TheiaCoV workflows
TheiaCoV_Illumina_PE_PHB: https://app.terra.bio/#workspaces/theiagen-training-workspaces/Theiagen_Otieno_Sandbox/job_history/7212fba6-b533-4afc-882f-a0aebc5a9157
TheiaCoV_Illumina_SE_PHB: https://app.terra.bio/#workspaces/theiagen-training-workspaces/Theiagen_Otieno_Sandbox/job_history/03a0096f-7636-47fe-be06-3446596f7daf
TheiaCoV_ONT_PHB: https://app.terra.bio/#workspaces/theiagen-training-workspaces/Theiagen_Otieno_Sandbox/job_history/11f05bb7-022c-4d44-914b-992614202e46

RSV and TheiaCoV_Illumina_PE_PHB
RSV-A: https://app.terra.bio/#workspaces/theiagen-training-workspaces/Theiagen_Otieno_Sandbox/job_history/8f315934-0771-40d9-914a-19abc7a9413d
best target for identification is "Human respiratory syncytial virus A"

  0.18	2637	2637	U	0	unclassified
 99.82	1499711	34	R	1	root
 99.78	1499055	11	D	10239	  Viruses
 99.77	1498888	0	D1	2559587	    Riboviria
 99.77	1498888	0	K	2732396	      Orthornavirae
 99.77	1498888	0	P	2497569	        Negarnaviricota
 99.77	1498888	0	P1	2497570	          Haploviricotina
 99.77	1498888	0	C	2497574	            Monjiviricetes
 99.77	1498888	0	O	11157	              Mononegavirales
 99.77	1498888	0	F	11244	                Pneumoviridae
 99.77	1498888	0	G	1868215	                  Orthopneumovirus
 99.77	1498887	0	S	3049954	                    Orthopneumovirus hominis
 99.77	1498887	967	S1	11250	                      human respiratory syncytial virus
 99.71	1497920	1497920	S2	208893	                        Human respiratory syncytial virus A

RSV-B: https://app.terra.bio/#workspaces/theiagen-training-workspaces/Theiagen_Otieno_Sandbox/job_history/73211a8e-21cb-4549-b424-01a7d636877a
best target for identification is "human respiratory syncytial virus"

1.58	6378	6378	U	0	unclassified
98.42	396488	9	R	1	root
98.36	396273	6	D	10239	  Viruses
98.31	396039	0	D1	2559587	    Riboviria
98.31	396039	1	K	2732396	      Orthornavirae
98.30	396037	0	P	2497569	        Negarnaviricota
98.30	396029	0	P1	2497570	          Haploviricotina
98.30	396029	0	C	2497574	            Monjiviricetes
98.30	396029	0	O	11157	              Mononegavirales
98.30	396029	1	F	11244	                Pneumoviridae
98.30	396028	2	G	1868215	                  Orthopneumovirus
98.30	396026	0	S	3049954	                    Orthopneumovirus hominis
98.30	396026	382062	S1	11250	                      human respiratory syncytial virus
 3.47	13964	13964	S2	208893	                        Human respiratory syncytial virus A

RSV and TheiaCoV_Illumina_PE_PHB with updated Kraken target organism identifier
RSV-A: https://app.terra.bio/#workspaces/theiagen-training-workspaces/Theiagen_Otieno_Sandbox/job_history/3546ab3d-c2cd-4c1e-ae3a-da78754733e7

RSV-B: https://app.terra.bio/#workspaces/theiagen-training-workspaces/Theiagen_Otieno_Sandbox/job_history/ce3d507f-05df-403f-812e-7f781f761364

Bacterial and Viral Pathogens and Kraken2 standalone workflows
Kraken2_PE_PHB: https://app.terra.bio/#workspaces/theiagen-training-workspaces/Theiagen_Otieno_Sandbox/job_history/ac5dc25b-a305-4ca2-be52-3845eb683d4e
Kraken2_SE_PHB: https://app.terra.bio/#workspaces/theiagen-training-workspaces/Theiagen_Otieno_Sandbox/job_history/fa7e5754-85de-4b34-929c-c11da23ab3d5
Kraken2_ONT_PHB: https://app.terra.bio/#workspaces/theiagen-training-workspaces/Theiagen_Otieno_Sandbox/job_history/6845c8eb-ee26-402f-ac11-550591f7a7ac

Suggested Scenarios for Reviewer to Test

🔬 Final Developer Checklist

The workflow/task has been tested and results, including file contents, are as anticipated
The CI/CD has been adjusted and tests are passing (Theiagen developers)
Code changes follow the style guide
Documentation and/or workflow diagrams have been updated if applicable (Theiagen developers only)

🎯 Reviewer Checklist

All changed results have been confirmed
You have tested the PR appropriately (see the testing guide for more information)
All code adheres to the style guide
MD5 sums have been updated
The PR author has addressed all comments
The documentation has been updated

…base for theiacov wfs

…sequencing platform

…om/theiagen/public_health_bioinformatics into jro-kraken-split-database-and-task

…ken DB used for theiacov workflows

… se test workflows

workflows/utilities/wf_organism_parameters.wdl

tasks/taxon_id/contamination/task_kraken2.wdl

workflows/theiacov/wf_theiacov_illumina_pe.wdl

workflows/theiacov/wf_theiacov_illumina_se.wdl

cimendes · 2024-10-21T10:56:24Z

I think this is looking pretty okay. It's a quite sizable PR as a lot of issues started to get pooled into the same PR. In short, the main changes are:

the kraken2 tasks just for theiacov went bye bye. Now all workflows use the kraken standalone task
New kraken2 viral+human database, constructed by theiagen (default viral doesn't include human which we report on)
task names, inputs and outputs have been harmonized
new logic implemented to only report % of sc2 if the organism is sc2. otherwise report % target organanism.

Additional tests that could be done that I personally didn't check but James did:

TheiaProk and TheiaMeta remain unaffected by these changes.

workflows/standalone_modules/wf_kraken2_pe.wdl

sage-wright · 2024-10-22T19:32:26Z

workflows/theiacov/wf_theiacov_illumina_pe.wdl

@@ -227,8 +230,8 @@ workflow theiacov_illumina_pe {
            num_reads_raw2 = read_QC_trim.fastq_scan_raw2,
            num_reads_clean1 = read_QC_trim.fastq_scan_clean1,
            num_reads_clean2 = read_QC_trim.fastq_scan_clean2,
-            kraken_human = read_QC_trim.kraken_human,
-            kraken_human_dehosted = read_QC_trim.kraken_human_dehosted,
+            kraken_human = read_QC_trim.kraken2_human,


could you update the qc_check_task to use kraken2 as well for consistency? ~~probably going to have to make this change in the export_taxon_table task as well~~

kraken2 is kraken2 everywhere now

workflows/theiacov/wf_theiacov_illumina_se.wdl

sage-wright · 2024-10-22T19:34:11Z

workflows/theiacov/wf_theiacov_clearlabs.wdl

@@ -37,6 +37,8 @@ workflow theiacov_clearlabs {
    String? target_organism


should this be kraken2_target_organism? i feel like we discussed this at one point in time and cannot remember the conclusion

There's a conflict with the output declaration in this particular case so I'm unsure how to proceed here. I0m leaving it as target_organism until we can discuss it further

on theiacov_ont is also target_organism

Would there be an instance we'd want this input to feed into multiple tasks, to ensure consistency? My gut reaction to having workflow level inputs be specific to tasks, is we should only do that if we could/would not use it in multiple workflows. This, to me, seems like an instance we might want to leave it without any tool name to allow for just that. Maybe worth some discussion? Ultimately, when we get to the PHB-lite effort, this will get abstracted away, but something to consider in the interim.

Pretty sure that's what we're using the organism_parameter workflow to do in order to standardize the language here. This input is piped into organism_parameter and that output is fed into all downstream tasks

Could you help me understand what you mean by having workflow level inputs specific to tasks if they're not used in multiple workflows?

If I'm understanding correctly, then we might be better served by removing all parameters that can be modified in the organism_parameters workflow so that they're not workflow-level inputs

sage-wright

Code looks great -- one question: if our aim is to standardize the use of kraken2 vs kraken, we should probably use kraken2 everywhere and not pick and choose.

Please go through and standardize all the kraken instances to kraken2 in both docs and all workflows & tasks.

sage-wright · 2024-10-22T19:46:26Z

Testing TheiaProk PE here, TheiaMeta here and Kraken2_PE here

cimendes · 2024-10-25T10:14:42Z

Other impacted workflows that require testing:

Freyja_FASTQ
NCBI_Scrub_PE
NCBI_Scrub_SE

…ew kraken2 task

cimendes · 2024-10-25T11:24:37Z

Okay, I think everything is ready for retesting!!

TheiaCoV
- PE
- SE
- ONT
TheiaMeta
Kraken2
- PE
- SE
- ONT
NCBI-SRUB
- PE
- SE
Freyja_FASTQ

AndrewLangvt

@cimendes I know this will be a pain to go back and refactor, but we need to avoid changing workflow-level output namespaces for modifications such as "harmonizing" the use of kraken/kraken2. i.e. have what is output to the Terra table remain the same, but we can change what defines that variable.
For example

public_health_bioinformatics/workflows/theiacov/updates/wf_ncbi_scrub_pe.wdl

Line 39 in 51b1c45

Float kraken2_human_dehosted = kraken2.kraken2_percent_human

Float kraken_human_dehosted = kraken2.percent_human
# should become 
Float kraken_human_dehosted = kraken2.kraken2_percent_human
# should NOT become 
Float kraken2_human_dehosted = kraken2.kraken2_percent_human

Changing all instances of kraken, including the output namespaces, introduces a backwards incompatible change that would pose some pain points to the end user. I know some of this may work may have begun with James, but let's ditch those, where possible and only make those changes "under the hood" so our users are unimpacted. Happy to discuss more, if you like.

jrotieno and others added 22 commits August 28, 2024 21:39

split kraken database and tool, use standalone task with default data…

fd3ed46

…base for theiacov wfs

renaming kraken2 task calls to just kraken instead of suffixing with …

428be7f

…sequencing platform

update output name

201e298

renaming kraken outputs to kraken2

3b99abf

additional kraken outputs

7f672a0

Merge branch 'main' into jro-kraken-split-database-and-task

0ea39a8

clearlabs outputs fix

7be05c0

Merge branch 'jro-kraken-split-database-and-task' of https://github.c…

0ca1515

…om/theiagen/public_health_bioinformatics into jro-kraken-split-database-and-task

updating RSV Kraken2 target organism identifiers and exposing the Kra…

9aa1681

…ken DB used for theiacov workflows

md5sum

88f2279

inputs to manage CI errors

f9a2ef9

CI error, again!

b838b15

adding a test kraken database for CI

e10df04

fix test theiacov inputs

c388970

md5sum

9e3eadf

optional target_organism for theiacov SE

bd6fba9

md5sum

8e8bc0b

new test database

d69d01d

updated test kraken database

a2a68fb

md5sum

644f69e

update CI for kraken2 report in theiacov clearlabs, ilmn pe, and ilmn…

912327c

… se test workflows

update CI

64a619e

cimendes reviewed Oct 7, 2024

View reviewed changes

workflows/utilities/wf_organism_parameters.wdl Outdated Show resolved Hide resolved

cimendes reviewed Oct 7, 2024

View reviewed changes

tasks/taxon_id/contamination/task_kraken2.wdl Show resolved Hide resolved

cimendes added 4 commits October 7, 2024 10:08

Merge branch 'main' into jro-kraken-split-database-and-task

258e5f3

update ci

0d73768

fiz ouput workflow name

07c081e

update docs - kraken2 standalone

c7925c4

cimendes reviewed Oct 7, 2024

View reviewed changes

workflows/theiacov/wf_theiacov_illumina_pe.wdl Outdated Show resolved Hide resolved

cimendes reviewed Oct 7, 2024

View reviewed changes

workflows/theiacov/wf_theiacov_illumina_se.wdl Outdated Show resolved Hide resolved

sage-wright reviewed Oct 22, 2024

View reviewed changes

workflows/standalone_modules/wf_kraken2_pe.wdl Outdated Show resolved Hide resolved

sage-wright reviewed Oct 22, 2024

View reviewed changes

workflows/theiacov/wf_theiacov_illumina_se.wdl Outdated Show resolved Hide resolved

sage-wright reviewed Oct 22, 2024

View reviewed changes

sage-wright requested changes Oct 22, 2024

View reviewed changes

cimendes added 14 commits October 25, 2024 09:23

Merge branch 'main' into jro-kraken-split-database-and-task

e9969fc

rename kraken2 outputs to match other theiacov, rename kraken2_db input

5fe7123

kraken2_db

88bc2f7

kraken2_db

bb90802

kraken2_db

fe5e93d

kraken -> kraken2

a28ff96

kraken -> kraken2

e529d19

more kraken -> kraken2

72dc913

kraken -> kraken2 continued

9e17028

more kraken -> kraken2

bcddf62

krakren -> kraken2

4b6d267

last kraken -> kraken2 (ignoring nullabor)

bc72236

fix output declaration

283d7e1

forgot about the ncbi_scrub standalone wfs again

6e7e0ef

cimendes added 2 commits October 25, 2024 11:00

update CI for theiaprok and making freyja_fastq functional with the n…

35e9d74

…ew kraken2 task

change output type

11d56dd

add missing pe

51b1c45

AndrewLangvt requested changes Oct 28, 2024

View reviewed changes

cimendes marked this pull request as draft November 4, 2024 19:10

cimendes closed this Nov 7, 2024

sage-wright deleted the jro-kraken-split-database-and-task branch November 7, 2024 20:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Kraken2] Split database from Kraken2 TheiaCoV task #608

[Kraken2] Split database from Kraken2 TheiaCoV task #608

jrotieno commented Sep 6, 2024 •

edited by cimendes

Loading

cimendes commented Oct 21, 2024

sage-wright Oct 22, 2024 •

edited

Loading

cimendes Oct 25, 2024

sage-wright Oct 22, 2024

cimendes Oct 25, 2024

cimendes Oct 25, 2024

AndrewLangvt Oct 28, 2024

sage-wright Oct 28, 2024 •

edited

Loading

sage-wright left a comment

sage-wright commented Oct 22, 2024 •

edited

Loading

cimendes commented Oct 25, 2024

cimendes commented Oct 25, 2024

AndrewLangvt left a comment

		@@ -37,6 +37,8 @@ workflow theiacov_clearlabs {
		String? target_organism

[Kraken2] Split database from Kraken2 TheiaCoV task #608

[Kraken2] Split database from Kraken2 TheiaCoV task #608

Conversation

jrotieno commented Sep 6, 2024 • edited by cimendes Loading

🧠 Summary

⚡ Impacted Workflows/Tasks

🛠️ Changes

⚙️ Algorithm

➡️ Inputs

⬅️ Outputs

🧪 Testing

Suggested Scenarios for Reviewer to Test

🔬 Final Developer Checklist

🎯 Reviewer Checklist

cimendes commented Oct 21, 2024

sage-wright Oct 22, 2024 • edited Loading

Choose a reason for hiding this comment

cimendes Oct 25, 2024

Choose a reason for hiding this comment

sage-wright Oct 22, 2024

Choose a reason for hiding this comment

cimendes Oct 25, 2024

Choose a reason for hiding this comment

cimendes Oct 25, 2024

Choose a reason for hiding this comment

AndrewLangvt Oct 28, 2024

Choose a reason for hiding this comment

sage-wright Oct 28, 2024 • edited Loading

Choose a reason for hiding this comment

sage-wright left a comment

Choose a reason for hiding this comment

sage-wright commented Oct 22, 2024 • edited Loading

cimendes commented Oct 25, 2024

cimendes commented Oct 25, 2024

AndrewLangvt left a comment

Choose a reason for hiding this comment

jrotieno commented Sep 6, 2024 •

edited by cimendes

Loading

sage-wright Oct 22, 2024 •

edited

Loading

sage-wright Oct 28, 2024 •

edited

Loading

sage-wright commented Oct 22, 2024 •

edited

Loading